Skip to content

[FLINK-38263][table] add secret store related interfaces #27394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
lihaosky wants to merge 9 commits into
base: master
Choose a base branch
from
Open

[FLINK-38263][table] add secret store related interfaces #27394

lihaosky wants to merge 9 commits into from

Conversation

@lihaosky
Copy link
Contributor

@lihaosky lihaosky commented Jan 8, 2026 •
edited
Loading

What is the purpose of the change

Add secret store related api in FLIP-529

Brief change log

  • SecretStore, SecretStoreFactory interfaces
  • Add secret-store kind config option
  • Use them in TableEnvironmentImpl and EnvironmentSettings

Verifying this change

Unit test

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (yes)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (yes)
  • If yes, how is the feature documented? (JavaDocs)

@flinkbot
Copy link
Collaborator

flinkbot commented Jan 8, 2026 •
edited
Loading

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Jan 9, 2026
twalthr
twalthr reviewed Feb 3, 2026
View reviewed changes
Copy link
Contributor

@twalthr twalthr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lihaosky.

...k-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java Outdated
// TODO (FLINK-38261): pass secret store to catalog manager for encryption/decryption
final SecretStore secretStore =
settings.getSecretStore() != null
? settings.getSecretStore()
Copy link
Contributor

@twalthr twalthr Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not discover a factory if a secret store is already provided. Please also update the CatalogStoreFactory above which has the same issue. Also the TableFactoryUtil class is kind of deprecated. Move the methods for both secret and catalog store to a new org.apache.flink.table.api.internal.ApiFactoryUtil

Copy link
Contributor Author

@lihaosky lihaosky Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a new PR for the refactoring: #27531

...table/flink-table-api-java/src/main/java/org/apache/flink/table/api/EnvironmentSettings.java Outdated

@Internal
@Nullable
public SecretStore getSecretStore() {
Copy link
Contributor

@twalthr twalthr Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Optional, maybe also for CatalogStore for consistency.

Copy link
Contributor Author

@lihaosky lihaosky Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put in new PR: #27531

...k-table-api-java/src/main/java/org/apache/flink/table/secret/GenericInMemorySecretStore.java Outdated
@Internal
public class GenericInMemorySecretStore implements ReadableSecretStore, WritableSecretStore {

private static final ObjectMapper objectMapper = new ObjectMapper();
Copy link
Contributor

@twalthr twalthr Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for Jackson deps here. We can store Java objects and avoid ser/de.

Copy link
Contributor

@twalthr twalthr Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store an immutable Map<String, String>

@lihaosky lihaosky mentioned this pull request Feb 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@snuyanzin snuyanzin snuyanzin left review comments

@twalthr twalthr twalthr left review comments

@davidradl davidradl davidradl left review comments

Assignees

No one assigned

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lihaosky @flinkbot @snuyanzin @twalthr @davidradl